Do not report cluster fields supplied by a cluster policy as drift - #6531
Merged
Conversation
Collaborator
Integration test reportCommit: c5a0b56
Top 6 slowest tests (at least 2 minutes):
|
denik
marked this pull request as ready for review
September 11, 2026 10:33
This was referenced Sep 11, 2026
janniklasrose
approved these changes
Sep 11, 2026
denik
force-pushed
the
denik/cluster-policy-no-drift
branch
3 times, most recently
from
September 14, 2026 09:54
afc8b25 to
282cd6b
Compare
Direct engine: a cluster field a cluster policy supplies server-side is no longer
reported as perpetual, unconvergeable drift.
- ignore_remote_additions rule (bundle/direct/dresources/resources.yml): inside a
cluster spec that sets policy_id, a field present in the remote but absent from
both state and config is skipped with reason remote_addition. A disagreement with
config, or a field removed from config, still reports update. Gated on policy_id
at clusters, jobs.job_clusters[*].new_cluster, jobs.tasks[*].new_cluster and
jobs.tasks[*].for_each_task.task.new_cluster. No rule for pipelines.clusters[*] --
the Pipelines API does not expand policies.
- when_set is a structpath.PathNode resolved relative to the matched object.
- libs/testserver applies cluster policies (fixed always; defaultValue only with
apply_policy_default_values; never overriding a supplied value; rejecting a value
contradicting a fixed element) so the tests run locally as they do on cloud.
- jobFixUps left apply_policy_default_values in ForceSendFields after zeroing it,
serializing an explicit false where the Jobs API returns nothing. Fixed.
The gate is policy_id, not apply_policy_default_values: fixed elements apply
regardless of that flag, and both reporters hit the bug with it unset.
Tests under acceptance/bundle/resources/{jobs,clusters,pipelines}/cluster_policy,
one condition each; unit tests for the classifier and testserver policy application;
a config test validating every rule's pattern and when_set gate.
Fixes #5179
Fixes #6512
Co-authored-by: Isaac
Only the 'Files: N uploaded' counts shifted (main changed the default uploaded file set); classification output is unchanged. Co-authored-by: Isaac
bundle config-remote-sync --save stopped copying genuine out-of-band remote changes (e.g. a spark_conf a user set through the API) into databricks.yml once those fields were classified remote_addition for the deploy planner: config-sync skips Skip changes, and remote_addition is Skip. - configsync/diff.go: process remote_addition entries (Action==Skip && Reason!=remote_addition is the new skip guard), so config-sync still captures them; its existing defaults.go filter drops policy-injected fields like custom_tags while keeping genuine ones. - bundle_plan.go: classify backend_default before remote_addition. The clusters remote_addition rule is root-level, so it was shadowing backend_default; with config-sync now reading the reason, backend defaults must keep their own reason to stay excluded from sync. New test: config-remote-sync/cluster_policy_remote_addition. Co-authored-by: Isaac
Add a comment at the classifier ladder and a unit test (TestLadderBackendDefaultBeforeRemoteAddition) pinning that backend_default is classified before remote_addition. The clusters remote_addition rule is root-level and matches every backend default, so the order is load-bearing for config-remote-sync (#6631); the test fails if the two branches are swapped. Co-authored-by: Isaac
Co-authored-by: Isaac
remote_addition is the broadest skip (any config-absent remote field in a gated object), so it belongs after the more specific skips, not right after backend_default. Behavior-neutral: mutually exclusive with normalized (needs New string) and missing_in_remote (implies Remote nil), and it still runs after backend_default and before recreate. Goldens unchanged; TestLadderBackendDefaultBeforeRemoteAddition still holds. Co-authored-by: Isaac
denik
force-pushed
the
denik/cluster-policy-no-drift
branch
from
September 14, 2026 10:41
282cd6b to
c5a0b56
Compare
ilyakuz-db
approved these changes
Sep 14, 2026
denik
enabled auto-merge
September 14, 2026 10:55
denik
added a commit
that referenced
this pull request
Sep 14, 2026
#6531 added this test on main; its golden was generated without this PR's per-resource reporting. Merged with this branch, the deploy now prints "Created cluster_policies.my_policy" inline before the dependent job-create fails (the policy is created first, in dependency order), so the golden needs that line. Deterministic (single resource before the failure), verified 0 failures across 10 runs of all five cluster_policy scenarios. Co-authored-by: Isaac <no-reply@databricks.com>
This was referenced Sep 14, 2026
denik
added a commit
that referenced
this pull request
Sep 14, 2026
#6531 added this test on main; its golden was generated without this PR's per-resource reporting. Merged with this branch, the deploy now prints "Created cluster_policies.my_policy" inline before the dependent job-create fails (the policy is created first, in dependency order), so the golden needs that line. Deterministic (single resource before the failure), verified 0 failures across 10 runs of all five cluster_policy scenarios. Co-authored-by: Isaac <no-reply@databricks.com>
denik
added a commit
that referenced
this pull request
Sep 14, 2026
#6531 added this test on main; its golden was generated without this PR's per-resource reporting. Merged with this branch, the deploy now prints "Created cluster_policies.my_policy" inline before the dependent job-create fails (the policy is created first, in dependency order), so the golden needs that line. Deterministic (single resource before the failure), verified 0 failures across 10 runs of all five cluster_policy scenarios. Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: c04c675
519 interesting tests: 415 MISS, 81 FAIL, 15 flaky, 4 RECOVERED, 3 KNOWN, 1 SKIP
Top 50 slowest tests (at least 2 minutes):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Direct engine: a cluster field that a cluster policy supplies server-side is no longer reported as perpetual drift.
ignore_remote_additionsrule (bundle/direct/dresources/resources.yml): inside a cluster spec that setspolicy_id, a field present in the remote but absent from both state and config is skipped with reasonremote_addition. A disagreement with config, or a field removed from config, still reports anupdate. Applied atclusters,jobs.job_clusters[*].new_cluster,jobs.tasks[*].new_cluster,jobs.tasks[*].for_each_task.task.new_cluster. No rule forpipelines.clusters[*]— the Pipelines API does not expand policies.backend_defaultis classified beforeremote_addition. The clustersremote_additionrule is root-level, so it matches every backend default; keeping backend defaults under their own reason is load-bearing for config-remote-sync (below), and a unit test pins the order.--savemust still capture genuine out-of-band remote additions (e.g. aspark_confa user set through the API) even though the deploy planner suppresses them as drift. config-sync now processesremote_additionentries (its existingdefaults.gofilter still drops policy-injected fields likecustom_tags). Supersedes fix: preserve config-sync of remote cluster additions #6631 — that fix and itsconfig-remote-sync/cluster_policy_remote_additiontest are Ilya's, cherry-picked with authorship.libs/testserverapplies cluster policies (fixedalways;defaultValueonly withapply_policy_default_values; never overriding a supplied value; rejecting a value contradicting afixedelement), plus a fix tojobFixUpsthat leftapply_policy_default_valuesinForceSendFieldsafter zeroing it.Why
A cluster policy supplies settings server-side, so the remote spec is a superset of what the bundle declares and every deploy planned an update that never converged.
The gate is
policy_id, notapply_policy_default_values:fixedpolicy elements are applied whether or not that flag is set, and both reporters hit the bug with it unset. Measured against a real workspace:apply_policy_default_valuesfixedfixedtruedefaultValuedefaultValuetrueThe reason is named
remote_addition, not "policy-managed": the rule only knows the field is a remote-only addition inside a policy-gated object — it does not verify the value came from the policy.Fixes #5179
Fixes #6512
Tests
One condition per test. Jobs and pipelines run on cloud (no cluster boot at deploy); the standalone
clustersresource boots a real cluster, so it isCloudSlow. All also run locally against the testserver.acceptance/bundle/resources/jobs/cluster_policy/:fixed_addition— afixedpolicy supplies an omitted scalar (spark_version) and map key (custom_tags) at all three jobs cluster-spec locations →remote_addition.default_flag—defaultValuesupplied only withapply_policy_default_values.config_conflicts_policy— config value contradicting afixedelement → 400 (asserted withmusterr).owned_tag_drift— out-of-band change / removal of a config-owned tag →update, policy tag skipped beside it.policy_id_scoping—policy_idgates suppression per cluster spec (gated → skip, ungated → update).acceptance/bundle/resources/clusters/cluster_policy/— standalone cluster (CloudSlow).acceptance/bundle/resources/pipelines/cluster_policy/— pipeline cluster: no expansion, converges.acceptance/bundle/config-remote-sync/cluster_policy_remote_addition/— config-sync captures a user'sspark_confwhile ignoring the policy tag.Plus unit tests for the classifier, the ladder order, and the testserver policy application, and a config test validating every rule's pattern and
when_setgate.This pull request and its description were written by Isaac.